Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add nio http transport to security plugin #32018

Merged
merged 7 commits into from
Jul 13, 2018
Merged

Add nio http transport to security plugin #32018

merged 7 commits into from
Jul 13, 2018

Conversation

Tim-Brooks
Copy link
Contributor

This is related to #27260. It adds the SecurityNioHttpServerTransport
to the security plugin. It randomly uses the nio http transport in
security integration tests.

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security

jaymode
jaymode approved these changes Jul 13, 2018
View reviewed changes
Copy link
Member

@jaymode jaymode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a couple of minor thing but otherwise LGTM

.../plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/SecurityRestFilter.java Outdated
ServerTransportFilter.extractClientCertificates(logger, threadContext, handler.engine(), nettyChannel);
SSLEngine sslEngine = SSLEngineUtils.getSSLEngine(httpChannel);

ServerTransportFilter.extractClientCertificates(logger, threadContext, sslEngine, httpChannel);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we move extractClientCertificates into SSLEngineUtils and just have a single method call here?

...plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/SSLEngineUtils.java Outdated

import javax.net.ssl.SSLEngine;

public class SSLEngineUtils {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a private constructor so no one instantiates this class

...y/src/main/java/org/elasticsearch/xpack/security/transport/SecurityHttpExceptionHandler.java Outdated
import static org.elasticsearch.xpack.core.security.transport.SSLExceptionHelper.isNotSslRecordException;
import static org.elasticsearch.xpack.core.security.transport.SSLExceptionHelper.isReceivedCertificateUnknownException;

public class SecurityHttpExceptionHandler implements BiConsumer<HttpChannel, Exception> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make it final

...lugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/NioIPFilter.java Outdated

import java.util.function.Predicate;

public class NioIPFilter implements Predicate<NioSocketChannel> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make it final

@Tim-Brooks
Copy link
Contributor Author

@jaymode Can you confirm those were the changes you wanted in regard to extractClientCertificates?

@jaymode
Copy link
Member

jaymode commented Jul 13, 2018

@tbrooks8 that was exactly what I was looking for 👍

@Tim-Brooks Tim-Brooks merged commit 305bfea into elastic:master Jul 13, 2018
@tylersmalley
Copy link
Contributor

Looks like this broke the Kibana CI, which builds from source. I expect we will also see this with the release manager tonight as well.

https://kibana-ci.elastic.co/job/elastic+kibana+master+multijob-x-pack/563/console

21:47:43    │ info  ./gradlew :distribution:archives:tar:assemble
21:47:43    │ERROR  Picked up JAVA_TOOL_OPTIONS: -Dfile.encoding=UTF8
21:48:06    │ERROR  Picked up JAVA_TOOL_OPTIONS: -Dfile.encoding=UTF8
21:48:07    │ERROR  WARNING: An illegal reflective access operation has occurred
21:48:07    │ERROR  WARNING: Illegal reflective access by org.codehaus.groovy.reflection.CachedClass (file:/var/lib/jenkins/.gradle/wrapper/dists/gradle-4.8.1-all/6fmj4nezasjg1b7kkmy10xgo2/gradle-4.8.1/lib/groovy-all-2.4.12.jar) to method java.lang.Object.finalize()
21:48:07    │ERROR  WARNING: Please consider reporting this to the maintainers of org.codehaus.groovy.reflection.CachedClass
21:48:07    │ERROR  WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
21:48:07    │ERROR  WARNING: All illegal access operations will be denied in a future release
21:48:11    │ERROR  warning: [options] bootstrap class path not set in conjunction with -source 8
21:48:12    │ERROR  1 warning
21:48:19    │ERROR  Picked up JAVA_TOOL_OPTIONS: -Dfile.encoding=UTF8
21:48:19    │ERROR  WARNING: An illegal reflective access operation has occurred
21:48:19    │ERROR  WARNING: Illegal reflective access by org.codehaus.groovy.reflection.CachedClass (file:/var/lib/jenkins/.gradle/wrapper/dists/gradle-4.8.1-all/6fmj4nezasjg1b7kkmy10xgo2/gradle-4.8.1/lib/groovy-all-2.4.12.jar) to method java.lang.Object.finalize()
21:48:19    │ERROR  WARNING: Please consider reporting this to the maintainers of org.codehaus.groovy.reflection.CachedClass
21:48:19    │ERROR  WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
21:48:19    │ERROR  WARNING: All illegal access operations will be denied in a future release
21:48:51    │ERROR  Note: /var/lib/jenkins/workspace/elastic+kibana+master+multijob-x-pack/elasticsearch/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContentGenerator.java uses or overrides a deprecated API.
21:48:51    │ERROR  Note: Recompile with -Xlint:deprecation for details.
21:49:29    │ERROR  Note: Some input files use or override a deprecated API.
21:49:29    │ERROR  Note: Recompile with -Xlint:deprecation for details.
21:49:29    │ERROR  Note: Some input files use unchecked or unsafe operations.
21:49:29    │ERROR  Note: Recompile with -Xlint:unchecked for details.
21:49:49    │ERROR  Note: Some input files use or override a deprecated API.
21:49:49    │ERROR  Note: Recompile with -Xlint:deprecation for details.
21:49:50    │ERROR  Note: /var/lib/jenkins/workspace/elastic+kibana+master+multijob-x-pack/elasticsearch/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/JsonProcessor.java uses unchecked or unsafe operations.
21:49:50    │ERROR  Note: Recompile with -Xlint:unchecked for details.
21:49:50    │ERROR  Note: Some input files use or override a deprecated API.
21:49:50    │ERROR  Note: Recompile with -Xlint:deprecation for details.
21:49:51    │ERROR  Note: /var/lib/jenkins/workspace/elastic+kibana+master+multijob-x-pack/elasticsearch/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/MustacheScriptEngine.java uses or overrides a deprecated API.
21:49:51    │ERROR  Note: Recompile with -Xlint:deprecation for details.
21:49:54    │ERROR  Note: Some input files use or override a deprecated API.
21:49:54    │ERROR  Note: Recompile with -Xlint:deprecation for details.
21:49:56    │ERROR  Note: Some input files use or override a deprecated API.
21:49:56    │ERROR  Note: Recompile with -Xlint:deprecation for details.
21:49:58    │ERROR  Note: Some input files use or override a deprecated API.
21:49:58    │ERROR  Note: Recompile with -Xlint:deprecation for details.
21:50:04    │ERROR  Note: Some input files use or override a deprecated API.
21:50:04    │ERROR  Note: Recompile with -Xlint:deprecation for details.
21:50:04    │ERROR  Note: Some input files use unchecked or unsafe operations.
21:50:04    │ERROR  Note: Recompile with -Xlint:unchecked for details.
21:50:10    │ERROR  Note: /var/lib/jenkins/workspace/elastic+kibana+master+multijob-x-pack/elasticsearch/x-pack/plugin/graph/src/main/java/org/elasticsearch/xpack/graph/action/TransportGraphExploreAction.java uses or overrides a deprecated API.
21:50:10    │ERROR  Note: Recompile with -Xlint:deprecation for details.
21:50:17    │ERROR  Note: Some input files use or override a deprecated API.
21:50:17    │ERROR  Note: Recompile with -Xlint:deprecation for details.
21:50:17    │ERROR  Note: Some input files use unchecked or unsafe operations.
21:50:17    │ERROR  Note: Recompile with -Xlint:unchecked for details.
21:50:23    │ERROR  Note: Some input files use or override a deprecated API.
21:50:23    │ERROR  Note: Recompile with -Xlint:deprecation for details.
21:50:24    │ERROR  Note: /var/lib/jenkins/workspace/elastic+kibana+master+multijob-x-pack/elasticsearch/x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/action/TransportRollupSearchAction.java uses or overrides a deprecated API.
21:50:24    │ERROR  Note: Recompile with -Xlint:deprecation for details.
21:50:28    │ERROR  /var/lib/jenkins/workspace/elastic+kibana+master+multijob-x-pack/elasticsearch/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioHttpServerTransport.java:62: error: method sslConfiguration in class SSLService cannot be applied to given types;
21:50:28    │ERROR              this.sslConfiguration = sslService.sslConfiguration(SSLService.getHttpTransportSSLSettings(settings), Settings.EMPTY);
21:50:28    │ERROR                                                ^
21:50:28    │ERROR    required: Settings
21:50:28    │ERROR    found: Settings,Settings
21:50:28    │ERROR    reason: actual and formal argument lists differ in length
21:50:28    │ERROR  /var/lib/jenkins/workspace/elastic+kibana+master+multijob-x-pack/elasticsearch/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SecurityNioHttpServerTransport.java:62: error: getHttpTransportSSLSettings(Settings) has private access in SSLService
21:50:28    │ERROR              this.sslConfiguration = sslService.sslConfiguration(SSLService.getHttpTransportSSLSettings(settings), Settings.EMPTY);
21:50:28    │ERROR                                                                            ^
21:50:28    │ERROR  Note: Some input files use or override a deprecated API.
21:50:28    │ERROR  Note: Recompile with -Xlint:deprecation for details.
21:50:28    │ERROR  Note: Some input files use unchecked or unsafe operations.
21:50:28    │ERROR  Note: Recompile with -Xlint:unchecked for details.
21:50:28    │ERROR  2 errors
21:50:28    │ERROR  
21:50:28    │ERROR  FAILURE: Build failed with an exception.
21:50:28    │ERROR  
21:50:28    │ERROR  * What went wrong:
21:50:28    │ERROR  Execution failed for task ':x-pack:plugin:security:compileJava'.
21:50:28    │ERROR  > Compilation failed; see the compiler error output for details.
21:50:28    │ERROR  
21:50:28    │ERROR  * Try:
21:50:28    │ERROR  Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights.
21:50:28    │ERROR  
21:50:28    │ERROR  * Get more help at https://help.gradle.org
21:50:28    │ERROR  
21:50:28    │ERROR  BUILD FAILED in 2m 44s
21:50:29 Error: unable to build ES

Tim-Brooks added a commit to Tim-Brooks/elasticsearch that referenced this pull request Jul 14, 2018
@Tim-Brooks
Fix compile issues introduced by merge
d4ecd97 
The build was broken due to some issues with the merging of elastic#32018. A
method that was public went private before the PR was merged. That did
not cause a merge conflict (so the PR was merged successfully). But it
did cause the build to fail.
@Tim-Brooks Tim-Brooks mentioned this pull request Jul 14, 2018
Merged
Tim-Brooks added a commit that referenced this pull request Jul 14, 2018
@Tim-Brooks
Fix compile issues introduced by merge (#32058)
a612404 
The build was broken due to some issues with the merging of #32018. A
method that was public went private before the PR was merged. That did
not cause a merge conflict (so the PR was merged successfully). But it
did cause the build to fail.
@Tim-Brooks Tim-Brooks deleted the nio_http_security branch December 18, 2019 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jaymode jaymode jaymode approved these changes

@s1monw s1monw Awaiting requested review from s1monw

@tvernum tvernum Awaiting requested review from tvernum

@jasontedor jasontedor Awaiting requested review from jasontedor

Assignees
No one assigned
Labels
>non-issue :Security/TLS SSL/TLS, Certificates v7.0.0-beta1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Tim-Brooks @elasticmachine @jaymode @tylersmalley @colings86